Feature/meeting with registry caserecord casefile support - #53

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docker-compose.yaml`:
- Around line 25-33: The docker-compose currently mounts the named volume
localstack_data:/var/lib/localstack which does not provide reliable S3
persistence in LocalStack Community Edition; either remove the localstack_data
volume to keep the compose file minimal if you intend to re-seed state on every
start, or document/implement a persistence solution: upgrade to LocalStack Pro
and set the PERSISTENCE env var to "1" (and update the service environment in
compose), or integrate a Community workaround such as GREsau/localstack-persist
and mount its hooks into the LocalStack container; reference the localstack_data
volume and the PERSISTENCE environment variable when applying the chosen change.

In `@src/main/java/backendlab/team4you/casefile/CaseFileService.java`:
- Around line 38-52: The tests in CaseFileServiceTest are missing a mock for
MeetingAgendaDocumentRepository which causes NPE when CaseFileService.deleteFile
calls meetingAgendaDocumentRepository.existsByCaseFileId(...); add a field
annotated with `@Mock` for MeetingAgendaDocumentRepository in CaseFileServiceTest
and in each of the three deleteFile test methods add a stub:
when(meetingAgendaDocumentRepository.existsByCaseFileId(anyLong())).thenReturn(false);
so the service method can run without hitting a null repository.

In
`@src/main/java/backendlab/team4you/exceptions/GlobalRestExceptionHandler.java`:
- Around line 209-213: The FileInUseException handler returns a raw String which
breaks the consistent error envelope; update GlobalRestExceptionHandler so
handleFileInUse(FileInUseException) returns ResponseEntity<ErrorResponseDto>
using the same ErrorResponseDto structure (status, error, message, timestamp) as
other handlers, or simply add FileInUseException.class to the existing
handleConflict `@ExceptionHandler` list; ensure you populate the ErrorResponseDto
fields the same way other handlers do and return
ResponseEntity.status(HttpStatus.CONFLICT).body(theDto).

In
`@src/main/java/backendlab/team4you/exceptions/GlobalViewExceptionHandler.java`:
- Line 12: Update GlobalViewExceptionHandler.handleNotFound to include the two
meeting-specific exceptions so they map to 404: add MeetingNotFoundException and
MeetingAgendaDocumentNotFoundException to the list/condition handled by
handleNotFound (the same place where other NotFound exceptions are checked) so
that when these exceptions are thrown by methods like showMeeting,
addAgendaItem, moveAgendaItemUp/Down, removeAgendaItem, populateMeetingsPage or
addAgendaDocument they are caught and return the existing 404 response instead
of falling through to handleUnexpected.

In `@src/main/java/backendlab/team4you/meeting/MeetingAgendaItemRepository.java`:
- Line 13: Remove the malformed repository method declaration findById(Meeting,
CaseRecord) from MeetingAgendaItemRepository because it conflicts with
JpaRepository's reserved findById(ID) signature and causes Spring Data startup
errors; delete that method line and keep relying on the existing JpaRepository
findById(Long) and the existsByMeetingAndCaseRecord method for any
meeting+caseRecord checks (note MeetingService uses findById(agendaItemId)),
then run the app/tests to verify Spring Data initializes cleanly.

In `@src/main/java/backendlab/team4you/meeting/MeetingController.java`:
- Around line 175-180: The catch blocks in MeetingController around calls to
meetingService.addCaseRecordToMeeting and removeAgendaItem are too narrow
(catching IllegalArgumentException) so domain exceptions like
DuplicateMeetingAgendaItemException, InvalidMeetingStateException, and
MeetingAgendaItemNotFoundException bubble up; update those handlers to catch the
specific domain exceptions (e.g., catch InvalidMeetingStateException |
DuplicateMeetingAgendaItemException | MeetingAgendaItemNotFoundException) or use
the same broader Exception strategy as used for
moveAgendaItemUp/moveAgendaItemDown and addAgendaDocument/removeAgendaDocument,
and ensure you set model.addAttribute("errorMessage", exception.getMessage()) so
the fragment renders the error consistently.
- Line 74: Multiple success messages in MeetingController are inconsistently
capitalized; update each model.addAttribute("successMessage", "...") call so the
message text uses sentence case (capitalize the first letter) for consistency.
Specifically change "sammanträdet skapades.", "sammanträdet uppdaterades.",
"sammanträdet togs bort.", "ärendet lades till på sammanträdet.", and
"dagordningspunkten togs bort." to start with an uppercase letter, keeping the
rest of the text unchanged; locate these strings in MeetingController.java (the
model.addAttribute("successMessage", ...) calls) and make the capitalization
edits across the create/update/delete/add/remove handlers to normalize UX
messaging.
- Around line 119-123: The catch block in MeetingController around updateMeeting
is too broad and re-fetching the meeting can re-throw MeetingNotFoundException
and hide the original error; change the error handling so you capture
exception.getMessage() into a local variable first, avoid a blanket
catch(Exception) (catch specific exceptions like MeetingNotFoundException
separately), and guard the call to meetingService.getMeetingById(meetingId) with
its own try/catch: if getMeetingById throws MeetingNotFoundException then add
the original message to the model and call populateMeetingsPage in a safe
fallback path (or redirect to the meetings list) instead of letting the re-fetch
bubble up; refer to updateMeeting, meetingService.getMeetingById,
populateMeetingsPage in your changes.

In `@src/main/java/backendlab/team4you/meeting/MeetingService.java`:
- Line 339: In MeetingService where InvalidMeetingStateException is thrown with
the message "Ärendet saknar diarum.", correct the typo in the user-facing string
to "Ärendet saknar diarium." by updating the exception message in the throw site
inside the MeetingService class so the displayed Swedish word is spelled
correctly.

In `@src/main/resources/db/migration/V19__create_meeting_tables.sql`:
- Around line 33-34: The uk_meeting_agenda_item_meeting_order unique constraint
can be violated by resequenceAgendaItems called from removeAgendaItem because
updates are batched until commit; either modify the migration
(V19__create_meeting_tables.sql) to make CONSTRAINT
uk_meeting_agenda_item_meeting_order DEFERRABLE INITIALLY IMMEDIATE, or change
resequenceAgendaItems/removeAgendaItem to use the same safe two-step pattern as
moveAgendaItemUp/moveAgendaItemDown (assign a temporary out-of-range value like
0, call saveAndFlush() for that row, then assign the final new order and
saveAndFlush()) so no intermediate unique-key collisions occur.

In `@src/main/resources/templates/fragments/admin-meetings.html`:
- Around line 97-100: The label "Registry:" is in English while the rest of the
fragment uses Swedish; update the static label text next to the span that binds
to selectedMeeting.registry.name to "Diarium" so it matches the create form and
UI language consistency (look for the <span
th:text="${selectedMeeting.registry.name}">Kommunstyrelsen</span> and its
surrounding <p> block and replace the "Registry:" text with "Diarium").

In `@src/test/java/backendlab/team4you/meeting/MeetingControllerTest.java`:
- Around line 86-93: Tests in MeetingControllerTest are inconsistent about CSRF:
ensure every mockMvc.perform POST to /admin/meetings includes a CSRF token so
they reflect SecurityConfig; update the POST invocations in the tests
createMeeting_shouldReturnFragmentAndSuccessMessage (the POST at lines 86–93),
addAgendaItem_shouldReturnFragmentAndSuccessMessage,
updateMeeting_shouldReturnFragmentAndSuccessMessage, and
deleteMeeting_shouldReturnFragmentAndSuccessMessage to include .with(csrf())
just like createMeeting_shouldReturnFragmentAndErrorMessage_whenServiceThrows
does, so all POST tests consistently simulate CSRF-protected requests.

---

Nitpick comments:
In `@src/main/java/backendlab/team4you/casefile/CaseFileService.java`:
- Around line 182-194: The current method reads s3Key before checking
meetingAgendaDocumentRepository.existsByCaseFileId(fileId) and deletes the DB
row (caseFileRepository.delete) before attempting s3Service.deleteFile, which
can lead to inconsistent state if S3 deletion fails; reorder to check
existsByCaseFileId(fileId) first, then capture s3Key, and move the S3 delete to
run after the successful DB commit (use
TransactionSynchronizationManager.registerSynchronization or an afterCommit
callback) so caseFileRepository.delete runs inside the transaction and
s3Service.deleteFile runs only after commit, logging and handling exceptions
from s3Service.deleteFile without causing the DB transaction to roll back.

In
`@src/main/java/backendlab/team4you/exceptions/InvalidMeetingStateException.java`:
- Around line 3-7: Add a cause-preserving constructor to the
InvalidMeetingStateException so callers can wrap underlying errors; specifically
add an overload InvalidMeetingStateException(String message, Throwable cause)
that calls super(message, cause). Apply the same pattern to
MeetingAgendaItemNotFoundException, DuplicateMeetingAgendaItemException, and
DuplicateMeetingAgendaDocumentException to preserve root causes when these
exceptions wrap lower-level failures.

In `@src/main/java/backendlab/team4you/meeting/Meeting.java`:
- Around line 45-86: The Meeting entity currently sets createdAt/updatedAt in
onCreate()/onUpdate() with `@PrePersist/`@PreUpdate; replace this manual handling
by annotating the createdAt and updatedAt fields with Spring Data JPA's
`@CreatedDate` and `@LastModifiedDate`, add
`@EntityListeners`(AuditingEntityListener.class) to the Meeting class, and remove
the onCreate/onUpdate lifecycle methods; ensure JPA auditing is enabled in the
application (e.g., `@EnableJpaAuditing` in a config) so timestamps are
auto-populated and compatible with future auditor tracking.

In `@src/main/java/backendlab/team4you/meeting/MeetingAgendaItem.java`:
- Around line 9-94: getDocuments() currently returns the mutable backing field
documents which allows external mutation; change it to return an unmodifiable
view (e.g. Collections.unmodifiableList(documents)) and add helper methods
addDocument(MeetingAgendaDocument doc) and removeDocument(MeetingAgendaDocument
doc) that manage the documents list and set/unset the bidirectional link (ensure
MeetingAgendaDocument.agendaItem is updated) so all mutations go through
controlled methods.

In `@src/main/java/backendlab/team4you/meeting/MeetingService.java`:
- Around line 140-141: Replace the use of IllegalArgumentException in
MeetingService when entities are not found with the appropriate domain-specific
not-found exceptions so the global handler maps to 404s: change the
registryRepository.findById(...).orElseThrow(() -> new
IllegalArgumentException(...)) to throw RegistryNotFoundException, the
caseRecordRepository.findById(...).orElseThrow(...) to throw
CaseRecordNotFoundException, and the
meetingAgendaDocumentRepository.findById(...).orElseThrow(...) to throw
MeetingAgendaDocumentNotFoundException (preserve or adapt the existing error
message/constructor signature of each exception); these changes should be
applied in the methods inside MeetingService where Registry, CaseRecord and
MeetingAgendaDocument lookups occur so controllers and
GlobalRestExceptionHandler handle them consistently.

In `@src/main/resources/db/migration/V19__create_meeting_tables.sql`:
- Around line 1-50: Add explicit indexes for the FK columns to avoid sequential
scans: create indexes on meeting(registry_id) (used by
findByRegistryOrderByStartsAtAsc/Desc), meeting_agenda_item(case_record_id) (for
efficient lookup/constraint checks), and meeting_agenda_document(case_file_id)
(used by existsByCaseFileId). Also ensure there are indexes on
meeting_agenda_item(meeting_id) and meeting_agenda_document(agenda_item_id) to
support findByMeetingOrderByAgendaOrderAsc, findByAgendaItem and ON DELETE
CASCADE (note UNIQUE constraints may already provide an index for the leading
column, but add standalone CREATE INDEX statements if those indexes are not
present).

In `@src/main/resources/templates/admin-layout.html`:
- Around line 18-25: The HTMX script tag in admin-layout.html currently loads
https://unpkg.com/htmx.org@1.9.12 without verification; update the <script>
element that loads htmx.org (the HTMX CDN include at the bottom of the template)
to include a Subresource Integrity (integrity="sha384-...") attribute and
crossorigin="anonymous" to pin and verify the file, or replace the CDN reference
by serving the htmx.org asset locally and updating the script src to the local
path to eliminate the external dependency.

In `@src/main/resources/templates/fragments/admin-sidenav.html`:
- Around line 39-47: The "Sammanträden" nav item currently reuses the calendar
icon class "fa-calendar-days" (the anchor with hx-get="/admin/meetings" and
inner text "Sammanträden"); change its <i> icon class to a distinct Font Awesome
icon such as "fa-gavel", "fa-users", "fa-handshake" or "fa-people-group" so it
doesn’t duplicate the "Bokningar" icon and users can visually differentiate the
two menu entries.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/backendlab/team4you/meeting/MeetingController.java`:
- Around line 169-232: The handlers addAgendaItem, moveAgendaItemUp,
moveAgendaItemDown (and similarly removeAgendaItem, addAgendaDocument,
removeAgendaDocument) call meetingService.getMeetingById(...) before the
try/catch so a MeetingNotFoundException can escape and break HTMX fragment
handling; move the getMeetingById call inside each method's try block (or
defensively resolve a registryId fallback as done in updateMeeting) so the catch
block can handle MeetingNotFoundException and still call
populateMeetingsPage(registryId, meetingId) with a sensible fallback; update
each referenced method (addAgendaItem, moveAgendaItemUp, moveAgendaItemDown,
removeAgendaItem, addAgendaDocument, removeAgendaDocument) to obtain Meeting or
registryId inside the try and keep populateMeetingsPage usage unchanged.
- Around line 75-85: The catch in createMeeting currently calls
populateMeetingsPage(model, registryId, null) which can re-throw when registryId
is invalid; wrap that call in its own try/catch (same pattern used in
updateMeeting) so that if meetingService.getMeetingsForRegistry(registryId)
throws RegistryNotFoundException or IllegalArgumentException you fallback to
populating the page without the registry (e.g., call populateMeetingsPage(model,
null, null) or otherwise call the getAllMeetings path) and preserve the original
model attribute "errorMessage" so the friendly fragment is rendered instead of a
500.

In `@src/main/java/backendlab/team4you/meeting/MeetingService.java`:
- Around line 138-150: Several not-found/null checks in MeetingService use
IllegalArgumentException causing 400 responses; replace them with the
appropriate 404/validation exceptions so HTTP statuses match other methods: in
getMeetingsForRegistry replace the IllegalArgumentException with
RegistryNotFoundException when registryRepository.findById(...) is empty; in
addCaseRecordToMeeting replace IllegalArgumentException("Ärendet hittades
inte.") with CaseRecordNotFoundException when the case record lookup fails; in
removeDocumentFromAgendaItem replace
IllegalArgumentException("Dokumentkopplingen hittades inte.") with
MeetingAgendaDocumentNotFoundException when the document-link lookup fails; and
in updateMeetingStatus replace IllegalArgumentException("Status måste anges.")
with InvalidMeetingStateException for null/invalid status to match other
null-argument validations.

In `@src/main/resources/templates/fragments/admin-meetings.html`:
- Around line 117-120: The template is rendering raw enum names via
${selectedMeeting.status} and using the enum value as the option label; update
the UI to show localized Swedish labels instead. Modify the MeetingStatus enum
to expose a display helper (e.g., getLabel() returning localized text) or add
message keys for each enum value in MessageSource, then update the template
places that reference selectedMeeting.status and the <option> labels (the span
showing the status and the status dropdown option text) to use either
selectedMeeting.status.label (or selectedMeeting.getStatus().getLabel()) or
`#messages`['meeting.status.' + selectedMeeting.status] (and similarly for the
option iteration) so both the detail view and the dropdown show localized
strings.

---

Nitpick comments:
In `@src/main/java/backendlab/team4you/meeting/MeetingService.java`:
- Around line 183-193: Replace the fragile nextAgendaOrder calculation that uses
countByMeeting(meeting) + 1 with a max-based approach: add a finder on
meetingAgendaItemRepository such as
findTopByMeetingOrderByAgendaOrderDesc(Meeting meeting) (or a `@Query` to return
COALESCE(MAX(agenda_order),0)), retrieve the current max agendaOrder for the
given meeting, compute nextAgendaOrder = max + 1, and use that value when
constructing the MeetingAgendaItem in MeetingService; also ensure any null
result from the finder is treated as 0 so the first item becomes 1.

In `@src/test/java/backendlab/team4you/casefile/CaseFileServiceTest.java`:
- Around line 652-673: Add an explicit verification that the in-use check was
invoked: in the test method
deleteFile_shouldThrowFileInUseException_whenFileIsUsedByMeetingAgendaDocument,
after asserting the FileInUseException, call
verify(meetingAgendaDocumentRepository).existsByCaseFileId(100L) to pin that the
meetingAgendaDocumentRepository.existsByCaseFileId check is what prevented
deletion; keep the existing never() verifications for caseFileRepository.delete
and s3Service.deleteFile.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/backendlab/team4you/meeting/MeetingController.java`:
- Around line 327-333: The helper populateMeetingsPageAfterMeetingAction
currently only catches MeetingNotFoundException but populateMeetingsPage calls
meetingService.getMeetingsForRegistry(registryId) which can throw
RegistryNotFoundException; update populateMeetingsPageAfterMeetingAction to also
catch RegistryNotFoundException (in the same catch block or an additional one)
and in that case call populateMeetingsPage(model, null, null) just like the
MeetingNotFoundException path so HTMX fragment flows don't break; reference
populateMeetingsPageAfterMeetingAction, populateMeetingsPage,
meetingService.getMeetingsForRegistry, RegistryNotFoundException, and
MeetingNotFoundException when making the change.
- Around line 75-88: The fallback logic incorrectly assumes
registryRepository.findById(registryId) can throw; replace the try/catch with an
existence check and only set safeRegistryId when the registry actually exists:
call registryRepository.findById(registryId).isPresent() (or use
existsById(registryId)) to decide whether to assign safeRegistryId, so
populateMeetingsPage receives null for unknown registries and avoid
re-triggering meetingService.getMeetingsForRegistry which throws
RegistryNotFoundException; keep the original model.addAttribute("errorMessage",
exception.getMessage()) and pass safeRegistryId into populateMeetingsPage.

In `@src/main/java/backendlab/team4you/meeting/MeetingService.java`:
- Around line 293-294: The code that resolves a CaseFile (the statement starting
with "CaseFile caseFile = caseFileRepository.findById(caseFileId) ...") is
throwing MeetingAgendaDocumentNotFoundException; replace that with the correct
CaseFileNotFoundException so the lookup throws new
CaseFileNotFoundException("Handlingen hittades inte.") (or equivalent localized
message) to match the existing GlobalRestExceptionHandler mapping and avoid
misleading logs and API responses.

In `@src/main/resources/templates/fragments/admin-meetings.html`:
- Around line 23-34: The label text for the registry select is inconsistent:
change the <label for="registryId">Instans</label> to use the same Swedish term
"Diarium" so it matches the select placeholder "Välj diarium" and the detail
view label "Diarium:"; update the label associated with the element id
"registryId" (and any related template fragment that references registryId if
present) to "Diarium" to ensure consistent UI terminology.

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/main/java/backendlab/team4you/meeting/MeetingService.java`:
- Around line 329-332: In MeetingService.updateMeetingStatus(Long meetingId,
MeetingStatus status) replace the thrown IllegalArgumentException for a null
status with InvalidMeetingStateException so null-validation is consistent with
other methods (lines that validate meetingId/status using
InvalidMeetingStateException); keep the same message text (or localize) and
ensure the check still occurs at the start of the method referencing the status
parameter and meetingId as before.

---

Nitpick comments:
In `@src/test/java/backendlab/team4you/meeting/MeetingServiceTest.java`:
- Line 61: The test fixture variable wrongCaseFile in MeetingServiceTest is
declared and initialized but never used; remove its declaration and the
corresponding initialization in the test setup (where wrongCaseFile is set) to
keep the test fixture minimal, ensuring you only keep variables actually
referenced by test methods (update any setup method that assigns wrongCaseFile
to avoid unused code).